-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(menu): update styles #15236
fix(menu): update styles #15236
Conversation
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@alisonjoseph this looks good! The only one thing is we still maintain the Here is the reference https://carbondesignsystem.com/components/menu/style#interactive-state-color |
@thyhmdo just updated the hover color to the primary tokens. Do you know what the max-height value should be? |
The only weird thing I see is here - seems the dropdown content doesn't stick to the menubutton - not sure if we care to address this or not 👀 @alisonjoseph Screen.Recording.2023-11-30.at.10.11.48.mov |
@andreancardona I see that too, unrelated to anything in this PR though |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
* fix(menu): update spacing and add title to menuitem * fix(menu): add max-height and scroll to menu * fix: set hover color menu to primary * chore: remove max-height value and revert storybook changes * chore: storybook example
Closes #14676
Changelog
New
MenuItem
Menu
Changed
MenuButton
andComboButton
$text-primary
to$text-secondary
, and explicityly set svgs inside to$icon-secondary
,Testing / Reviewing
Check Menu, MenuButton and ComboButton that they display correctly with updated tokens and that long text that overflows shows title on hover.
Review scrollbar on MenuButton default story